fix(security): URL parameter injection and redirect URI validation#45
fix(security): URL parameter injection and redirect URI validation#45Copilot wants to merge 8 commits into
Conversation
Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
- Fix URL parameter injection in client.py: URL-encode fire_id in
get_fire_overview() to prevent a compromised API server from injecting
extra query parameters via special characters in FireId values.
- Fix overly-permissive redirect URI check in b2c_login.py: replace
the broad `startswith('msal') and '://auth' in location` check with
`startswith(f'msal{CLIENT_ID}://auth?')` to prevent acceptance of
redirects to attacker-controlled domains like msal{ID}://auth.evil.com
or wrong-client-ID redirects.
- Add security regression tests for both vulnerabilities.
Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
|
@copilot why did you make changes to pyproject.toml? |
These files were modified by running `uv add --dev` during environment setup to install test tools that were already in [project.optional-dependencies].dev. The additions to [dependency-groups].dev were not intentional. Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
Those changes were an unintended side effect of running |
|
@copilot fix ci errors |
The only CI failure is the Conventional Commits check. The PR title Could you rename the PR title to something like:
I'm unable to update the PR title directly — that's a GitHub API write operation outside my available tools. |
deviantintegral
changed the title
|
@copilot check again, the lint-and-test job is failing with ruff errors. |
Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
|
@copilot i see the uv.lock changes. I know it's unrelated to security but lets fix it in this PR. Is it normal for the version of the project itself to be in uv.lock? If so, is there something we need to do with our release-please configuration to keep it updated automatically when new releases are made? |
… bumps Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
Yes, it's completely normal — uv always locks the root package itself (as an editable install) with its version from Fixed in 3c86ddc:
|
|
@copilot Will the release-please configuration for uv.lock affect child dependencies that happen to have the same version? Or is it already able to filter to just this library? |
The generic search pattern `version = "${version}"` would also match
child dependencies at the same version (e.g. distlib 0.4.0), corrupting
the lockfile. Instead, add a workflow that runs `uv lock` on
release-please PRs to regenerate the lockfile properly.
Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
Good catch — the Fixed in 9e70805: removed the |
|
@copilot this PR breaks the login flow. Here is a partial debug log of trying to log in. |
The redirect URI check was too strict — it required the query string immediately after 'auth' (://auth?), but B2C can also send ://auth/?... with a trailing slash. Now accepts both forms while still rejecting ://auth.evil.com by requiring the character after 'auth' to be '?' or '/'. Co-authored-by: deviantintegral <255023+deviantintegral@users.noreply.github.com>
... The redirect URI prefix was Fixed in 7feb180: the check now uses Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
fire_idinclient.pyb2c_login.py://auth/?variant (trailing slash before query string)pyproject.tomlanduv.locktests/test_b2c_login.py(CI lint-and-test failure)uv.lockversion mismatch (was0.3.0, now correctly reflects0.4.0frompyproject.toml)release-please-config.json,.release-please-manifest.json, andupdate-uv-lock.ymlworkflow to keepuv.lockin sync✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.